Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide tab bar #12395

Merged
merged 17 commits into from
Jan 29, 2025
Merged

Hide tab bar #12395

merged 17 commits into from
Jan 29, 2025

Conversation

priyanshu16095
Copy link
Contributor

@priyanshu16095 priyanshu16095 commented Jan 17, 2025

Fixes #9971

This PR introduces a feature to hide the tab bar when only a single library is open in JabRef, enhancing the user interface for devices with low screen space.

THE CHANGES MADE

  • When a single library is open, the tab bar is hidden.
  • Ensured that the tab bar is restored when more than one library is open, maintaining expected functionality.
  • Added the right-click context menu option to the menu > library to retain the functionality.
  • I own the copyright of the code submitted and I licence it under the MIT license
  • Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

Screenshot (250)

@priyanshu16095
Copy link
Contributor Author

This PR resolves the issues from the previous ones and has passed all the checks.
I have also learned about Git branching.

@priyanshu16095
Copy link
Contributor Author

priyanshu16095 commented Jan 17, 2025

@calixtus Should I add the option to turn this behavior on or off here?

Screenshot (252)

@calixtus
Copy link
Member

I would put it rather in workspace preferences. The entry table preferences should focus on the maintable. The tabbedPane itself is part of the frame.

@calixtus
Copy link
Member

Do not close a pull request in GitHub. Just push to your branch, GitHub will automatically update your pull request. This way all the discussion in the former pr is gone.

@calixtus
Copy link
Member

Continues #12394 and #12393

@calixtus
Copy link
Member

Do not forget to add a changelog entry

@priyanshu16095
Copy link
Contributor Author

priyanshu16095 commented Jan 17, 2025

Sure, I will not close the PR. By mistake, in the previous PRs, I also added the commits from the copyToOption branch.
I find no other option but to create a new PR.
I will make sure not to repeat this in the future.

@priyanshu16095 priyanshu16095 changed the title Hide Tab bar for single library view Hide tab bar Jan 17, 2025
@priyanshu16095
Copy link
Contributor Author

priyanshu16095 commented Jan 18, 2025

Screenshot (258)

Is the preference correctly placed in the UI?

Created a CSS class that can be added or removed.
Added a preference to control this behavior. (in workspace preference)

Using BindingHelper.subscribeFuture() is not working smoothly.
If no library is present and the preference is set to true, the tab bar is still displayed.
After adding and removing a library, it starts working as expected.

Not using BindingHelper.subscribeFuture() works fine, but if anyone changes the preference, it does not reflects in real time and shows from the next time.

Please review.

@priyanshu16095
Copy link
Contributor Author

priyanshu16095 commented Jan 18, 2025

@calixtus The right click context menu has six options:

  1. Library Properties:
    can also be accessed from Menu > Library > Library Properties

  2. Reveal in file explorer

  3. Open terminal here

  4. Close library:
    can also be accessed from Menu > File > Close Library

  5. Close others:
    No use if there is only a single library

  6. Close all:
    Option 4 will function the same

The functionality of option 2 and 3 would be lost.
However, they can be added in the Menu > Library.

A three-dot menu can be placed in the top right of main table to show the same right-click context menu.

@priyanshu16095
Copy link
Contributor Author

priyanshu16095 commented Jan 18, 2025

Screenshot (261)

@calixtus Should the close button be placed here?

@calixtus
Copy link
Member

Is the preference correctly placed in the UI?

Yes, i like it.

Created a CSS class that can be added or removed. Added a preference to control this behavior. (in workspace preference)

Using BindingHelper.subscribeFuture() is not working smoothly. If no library is present and the preference is set to true, the tab bar is still displayed. After adding and removing a library, it starts working as expected.

Not using BindingHelper.subscribeFuture() works fine, but if anyone changes the preference, it does not reflects in real time and shows from the next time.

Maybe you can use a PseudoClass to distinguish between hidden and shown.

Did you try EasyBind.subscribe()?
Maybe you have to store the Subscription as a class variable to keep it from beeing removed by the garbage collector.

@calixtus
Copy link
Member

@calixtus The right click context menu has six options:

1. Library Properties:
   can also be accessed from Menu > Library > Library Properties

2. Reveal in file explorer

3. Open terminal here

4. Close library:
   can also be accessed from Menu > File > Close Library

5. Close others:
   No use if there is only a single library

6. Close all:
   Option 4 will function the same

The functionality of option 2 and 3 would be lost. However, they can be added in the Menu > Library.

A three-dot menu can be placed in the top right of main table to show the same right-click context menu.

@JabRef/developers wdyt?

@Siedlerchr
Copy link
Member

I would move all the options to the library menu options. This has the advantage that it can also be shown when you have multiple tabs.

@calixtus
Copy link
Member

I would move all the options to the library menu options. This has the advantage that it can also be shown when you have multiple tabs.

Move? Why? We wanted the right-click menu when we introduced oneandahalf year ago.
Question was: do we want an additional three dot menu or just rely on the menu, when only one library is open?

@Siedlerchr
Copy link
Member

When one library is open we rely on the menu. I find another 3 dot thing confusing

@priyanshu16095
Copy link
Contributor Author

priyanshu16095 commented Jan 20, 2025

The feature is working fine, it observes both the number of open databases and reflects real-time changes based on the value of the checkbox.

Please review

Recording.2025-01-20.mp4

@priyanshu16095
Copy link
Contributor Author

priyanshu16095 commented Jan 22, 2025

I am unable to add the 'Open in File Explorer' option because the OpenDatabaseFolder class inside JabRefFrame is private.

private class OpenDatabaseFolder extends SimpleCommand {

@HoussemNasri
Copy link
Member

We can just make it public and static so we can create instances of it without needing JabRefFrame

@priyanshu16095
Copy link
Contributor Author

priyanshu16095 commented Jan 22, 2025

Is the feature complete, or does it also require a close button?

Recording.2025-01-22.mp4

@HoussemNasri
Copy link
Member

Protip: videos can drag & dropped in GitHub comments and they will be uploaded automatically :)

I don't know whether this is complete or not, Carl and Chris would be of a better help. But this reviewable at least so let's undraft it.

@HoussemNasri HoussemNasri marked this pull request as ready for review January 22, 2025 13:53
Siedlerchr
Siedlerchr previously approved these changes Jan 27, 2025
CHANGELOG.md Outdated Show resolved Hide resolved
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jan 27, 2025
@Siedlerchr
Copy link
Member

Feels a bit weird to me, and I would like to have a preference in the UI to enable/disable this feature

@priyanshu16095
Copy link
Contributor Author

This feature includes a preference (workspace preference) to enable or disable it.

@Siedlerchr
Copy link
Member

Oh totally overlooked it, okay so it's fine from my side. Requires a second review

Co-authored-by: Christoph <[email protected]>
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You modified Markdown (*.md) files and did not meet JabRef's rules for consistently formatted Markdown files.
To ensure consistent styling, we have markdown-lint in place.
Markdown lint's rules help to keep our Markdown files consistent within this repository and consistent with the Markdown files outside here.

You can check the detailed error output by navigating to your pull request, selecting the tab "Checks", section "Tests" (on the left), subsection "Markdown".

CHANGELOG.md Outdated Show resolved Hide resolved
src/main/java/org/jabref/gui/TabBarManager.java Outdated Show resolved Hide resolved
subhramit
subhramit previously approved these changes Jan 28, 2025
Copy link
Member

@subhramit subhramit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolve the changelog conflict (merge main, changes will be ordered as per chronology)

subhramit
subhramit previously approved these changes Jan 28, 2025
Copy link
Member

@subhramit subhramit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, I resolved it.
I also made some changes to your last two changelog entries. Please take a look at commit d8a242f.

Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good so far, just one last thing, then its good to go from my side.
Thanks!

src/main/java/org/jabref/gui/frame/JabRefFrame.java Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
@calixtus calixtus added this pull request to the merge queue Jan 29, 2025
@calixtus
Copy link
Member

@subhramit an me approved, so I'm moving this to merge workflow.
Thank you for your contribution.

Merged via the queue into JabRef:main with commit d1ce450 Jan 29, 2025
23 checks passed
@Siedlerchr Siedlerchr mentioned this pull request Feb 2, 2025
7 tasks
@koppor
Copy link
Member

koppor commented Feb 2, 2025

This PR caused an issue #12442, which was quickly fixed by one of the JabRef maintainers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide Tab bar if there is only a single library open
6 participants